Repository navigation
Pass the TestKit coverage agent on the command line, move the shared TestKit directory - #207
alexander-yevsyukov wants to merge 16 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…estKit home The harness used to attach the JaCoCo agent by writing `org.gradle.jvmargs` into a `gradle.properties` of a per-module TestKit directory. That tied the directory to one module, and on CI it was empty on every run, so the first worker build of each module downloaded its dependencies and generated the Gradle API JARs again. The agent now goes into the runner arguments as `-Dorg.gradle.jvmargs`, and the consuming build may name a TestKit directory shared by all modules via the optional `io.spine.tools.gradle.testkit.coverage.home` system property. Without it, each module keeps a directory of its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8549704c0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #207 +/- ##
============================================
+ Coverage 92.03% 92.12% +0.09%
- Complexity 846 848 +2
============================================
Files 160 161 +1
Lines 3164 3175 +11
Branches 381 384 +3
============================================
+ Hits 2912 2925 +13
Misses 122 122
+ Partials 130 128 -2 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approved
No active review issues are reported for the TestKit coverage harness changes.
No open issues 🎉
What changed in this PR
Updates plugin-testlib’s TestKit coverage harness to pass the JaCoCo agent through runner arguments, allowing worker builds to share a cacheable TestKit home without module-specific configuration files.
Changes:
- Attaches coverage through build helper arguments and adds optional shared-home selection while preserving caller precedence and the per-module fallback.
- Adds tests for argument generation, disabled coverage, and directory selection; updates consumer documentation.
- Bumps the snapshot version, refreshes dependency reports, and incorporates shared-home build configuration.
| File | Description |
|---|---|
version.gradle.kts |
Bumps the version to 2.0.0-SNAPSHOT.452. |
plugin-testlib/src/test/kotlin/io/spine/tools/gradle/testing/TestKitCoverageSpec.kt |
Tests coverage arguments and TestKit directory selection. |
plugin-testlib/src/main/kotlin/io/spine/tools/gradle/testing/TestKitCoverage.kt |
Moves agent configuration to arguments and supports a shared home. |
plugin-testlib/src/main/kotlin/io/spine/tools/gradle/testing/GradleProject.kt |
Applies coverage arguments and documents setup and direct-runner behavior. |
plugin-testlib/src/main/kotlin/io/spine/tools/gradle/testing/Gradle.kt |
Applies coverage arguments in runGradleBuild. |
docs/dependencies/pom.xml |
Updates the reported project version. |
docs/dependencies/dependencies.md |
Refreshes report versions and generation timestamps. |
buildSrc/src/main/kotlin/io/spine/gradle/testing/TestKitCoverage.kt |
Supplies the shared TestKit home system property. |
.github/workflows/publish.yml |
Updates the shared publishing workflow revision. |
Review effort: Balanced
Move the three system-property names into `public object TestKitCoverage`, so that the spec and the `GradleProject` KDoc refer to them instead of repeating the strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The worker builds generate no Gradle API JARs and download few files. Their Gradle user home mostly holds the instrumented JARs of the plugin classpath. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Remove `TESTKIT_HOME_PROPERTY` and the per-module `testkit-home` fallback. With the agent on the command line, the coverage harness needs no TestKit directory of its own. TestKit takes the one named by Gradle's `org.gradle.testkit.dir` system property, or a folder under the `Test` task's temporary directory, and setting that property is up to the consuming build, apart from enabling coverage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The file also picks up the current license header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`withSharedTestKitDirectory()` now takes an optional `dir`. Given none, a project uses `caches/spine-testkit` under the Gradle user home, which `GradleUserHome.kt` looks up as Gradle does: the `gradle.user.home` system property, then `GRADLE_USER_HOME`, then `.gradle` in the user's home. Remove `RootProject.testKitTempDir()`, which put the directory under the repository root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop the default arguments of `gradleUserHome()`, which nothing used, make the explicitly given TestKit directory private, and test the error for an unknown `user.home`. Codecov counted these as uncovered lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Two problems with the TestKit directory, the Gradle user home of the builds that
plugin-testlibruns through Gradle TestKit:org.gradle.jvmargsinto agradle.propertiesin the TestKit directory, so it had to choose that directory itself,build/testkit-homeper module. On CI it is empty on every run, so the first TestKit builds of each module redo the work a Gradle user home keeps, chiefly Gradle's instrumented copies of the JARs on the classpath of the plugin under test.withSharedTestKitDirectory()put it under the repository root, in.gradle-test-kit. Incore-jvm-compiler, that directory takes 1.76 GB in local tests, and every new worktree, of the same repository or another one, fills its own copy from scratch. That is costly when several agent sessions work in different worktrees or repositories on one machine.This PR:
withTestKitCoverage()appends-Dorg.gradle.jvmargs=-javaagent:…to the runner arguments for builds started byexecuteTask,executeAndFailandrunGradleBuild. The harness no longer chooses a TestKit directory, so coverage and the directory are independent of each other.withSharedTestKitDirectory(). It takes an optionaldir: File?. A given directory is used as is. Given none, the project usescaches/spine-testkitunder the Gradle user home. The newGradleUserHome.ktlooks the Gradle user home up in the order Gradle uses: thegradle.user.homesystem property, then theGRADLE_USER_HOMEenvironment variable, then.gradlein the user's home directory. It composes the path withFile, so Windows paths such asD:\a\.gradlework.@JvmOverloadskeeps the method without arguments.RootProject.testKitTempDir(), the old<repository root>/.gradle-test-kit. This is the breaking change.TestKitCoverage.AGENT_PROPERTYandEXEC_DIR_PROPERTYhold them, so the spec and the KDoc refer to the constants instead of repeating the strings. config'sbuildSrckeeps its own copies, because it does not depend on this library.plugin-testlibtoo. Its own tests now run with the agent like the other four TestKit modules. Today this adds no coverage, because the fixtures ofGradleProjectSpecrun only thejavaplugin, but it keeps every TestKit module on the same setup.config, with Say what a shared TestKit home saves config#833 and Set the TestKit directory apart from TestKit coverage config#834.useSharedTestKitDir()now sets Gradle'sorg.gradle.testkit.dirtocaches/spine-testkiton everyTesttask, apart fromenableTestKitCoverage(), so the other TestKit builds of all modules share the directory too. The pull also updates the versions pinned for Spine's own libraries.TestKitCoverageSpecfor the appended argument and the no-op cases,GradleUserHomeSpecfor the lookup, with two tests that run only on Windows, and new cases inGradleProjectSetupSpecfor the choice of the directory.GradleProjectKDoc: the consumer recipe for coverage, and a separate section on the TestKit directory.2.0.0-SNAPSHOT.460, as a breaking change, and updates the dependency reports.An earlier revision added an
io.spine.tools.gradle.testkit.coverage.homeproperty for a shared directory, which SpineEventEngine/config#831 set. It duplicated Gradle'sorg.gradle.testkit.dirand was removed, here and in config#834.Compatibility
RootProject.testKitTempDir()is removed.withSharedTestKitDirectory()still compile and link. They now sharecaches/spine-testkitunder the Gradle user home instead of keeping.gradle-test-kitin each checkout. In the org, these arecompiler(4 specs),core-jvm-compiler(4 integration tests) andProtoTap(1 spec).-g: the test JVM does not see it, so the default falls back toGRADLE_USER_HOMEor~/.gradle. That only loses the sharing with that build's caches.plugin-testlibversions callwithTestKitDir(<module>/build/testkit-home)themselves when coverage is on, which overridesorg.gradle.testkit.dir, so they keep their per-module directories.runnerdirectly:compilersets the environment and then callsexecuteTask, so it still gets the agent.ProtoTapandcore-jvm-compilercallwithJvmArguments(...), and explicit JVM arguments win overorg.gradle.jvmargsfrom any source. Those tests got no worker coverage before this change either.org.gradle.jvmargs, as the user-homegradle.propertiesdid before.plugin-testlib'stesttask is no longer cacheable, like those of the other four modules, so it reruns afterclean.Measurements
Size:
core-jvm-compiler's.gradle-test-kittakes 1.76 GB in local tests. Intool-base, thecaches/part of the shared directory takes about 250 MB, where per-module directories added up to 348 MB.Local wall-clock of the tests of the four coverage modules, measured with the earlier revision:
Windows PR run 41670107177, with the shared directory still empty: the build step took 733 s, against 837–901 s on the earlier runs that started with no cache, and no cache-lock timeouts.
Coverage: the root Kover report with the TestKit exec files covers 3035 of 3157 lines and 989 of 1188 branches, against 2943 and 970 without them. Codecov reported 92.08% for 4c7ec60, against 92.03% on
master, with misses unchanged at 122.Test plan
./gradlew build dokkaGenerate, locally, after deleting the repository-root.gradle-test-kitand every per-module TestKit directory. None came back, and the TestKit builds wrote into~/.gradle/caches/spine-testkit.GradleUserHomeSpec: 5 tests pass on macOS. The 2 Windows-only tests are skipped there.GradleUserHomeSpec.GradleProjectSetupSpec,GradleProjectSpec,TestKitCoverageSpecandRootProjectSpecpass.build/jacoco-testkit: each of the four coverage modules wrote itstestkit.exec(0.9–1.1 MB).plugin-testlib's coverage on, no class in the root or the module Kover report changed.spine-code-review,kotlin-engineer,review-docs. The latest changes, the Gradle user home lookup and the newwithSharedTestKitDirectory(), have not been re-reviewed.masterrun savescaches/spine-testkit, and later PR runs restore it.🤖 Generated with Claude Code